Skip to content

[SYCL][E2E] Build test output to separate files #15631

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 16 commits into from
Oct 11, 2024

Conversation

ayylol
Copy link
Contributor

@ayylol ayylol commented Oct 8, 2024

Currently, many tests that need to check executables built in different ways will build over the same output file. This patch edits these tests to instead build each executable to a different output file.

This should not have any functional differences in behavior, however it will help us later if we want to cache the built executables to run seperately.

@ayylol ayylol requested review from a team as code owners October 8, 2024 14:46
@ayylol ayylol requested a review from aelovikov-intel October 8, 2024 14:46
Copy link
Contributor

@sarnex sarnex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, but if we are going to rely on this for some CI optimizations we prolly need some CI check to make sure nobody adds a new test reusing the same file

@ayylol
Copy link
Contributor Author

ayylol commented Oct 8, 2024

lgtm, but if we are going to rely on this for some CI optimizations we prolly need some CI check to make sure nobody adds a new test reusing the same file

A note, the test DeviceLib/cmath_test.cpp never ran the build line wrapped with %if cuda (%if any-device-is-cuda is supposed to be used in this case) so the following line ran an executable that had been built by a previous command. If we implement a test like that we can avoid such situations.

@againull
Copy link
Contributor

This PR includes only trivial E2E test changes, so merging with failed test-perf (AMD system) task.

@againull againull merged commit d9122de into intel:sycl Oct 11, 2024
13 of 14 checks passed
@ayylol ayylol deleted the multiple-build branch October 11, 2024 19:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants